-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add --with-doas-confdir feature #71
base: master
Are you sure you want to change the base?
Conversation
Sure, I can swap the order. |
With supporting both files you have the conflict with Edit: Ah I see this was changed to build time, maybe it would be nice to support both if build with configuration directory support based on if |
Can you please clarify explicitly what you want us to do here? A user can already migrate their configuration by simply placing it in /etc/doas.d. |
|
Yes I think as @ericonr describes it would be the best, this avoids any breaking changes even with the build time option enabled. |
Ugh. That approach breaks our intended usecase for /etc/doas.d (restarting services from maintainer scripts). I would rather just migrate the configuration with a maintainer script. |
Would preferring |
I guess that would be ok too. I just want to avoid having multiple versions of the "same" program that are configured differently, but we can't really avoid that for this feature as long as upstream does not support it so at least not completely breaking compatibility if the distribution decides to enable this option is good IMHO. |
Okay, give me a minute, and I'll do that and qsort the files. |
Okay, both of those requests have been implemented and tested in d4527fe. |
60eadab
to
1157d84
Compare
I squashed everything down, because git was being screwy :) |
This adds support for an /etc/doas.d configuration directory as discussed in Duncaen#61. It is disabled by default.
1157d84
to
7de1d45
Compare
@Duncaen I think this is the final version, what do you think? |
I still don’t understand what’s the problem here.
How is that better than using both Consider this: you have If you really want one or another (I still don’t understand why), then it would be better do it in build-time: if doas was built with |
@jirutka in terms of Alpine, it does not matter, because the maintainer-script will automatically migrate |
No, it does matter, because user can still create As I said in the last paragraph, if @Duncaen and @ericonr wants one-or-another, for whatever reason, then I propose to do this decision in build-time based on |
I don't follow: once the configuration directory version of doas is deployed in Alpine, the maintainer script will move If an end user decides to remove the symlink, then it becomes a matter of educating the user about the migration to configuration directories. |
I’m talking about the correct behaviour of the feature in the terms of general user expectations based on the behaviour of all other programs with the same or similar feature, “common sense” and principle of least surprise. In general, not in Alpine only. You’re talking about mitigating or workarounding the problem/confusion created by the design and specifically about migration strategy for Alpine. Just tell me about one software that behaves in the same way as the currently accepted solution.
|
|
Regarding priority between file and folder, managing a |
Yes, that’s exactly what I originally proposed. But implementing support for |
Can we stop bikeshedding the design part and get back on topic? I implemented it according to how duncaen wanted it. |
@Duncaen anything left to do? i've tested both with config dir and with legacy config file and everything works as i would expect |
Looked good so far, I'm going to do some testing and another review over the weekend. Only missing part at the moment would maybe be a man page for |
@Duncaen, another week passed, did you find some time to do some testing and finish the review? |
It's still missing the doas.d manpage, I'll try to get that done this week. |
@Duncaen manpage done. I intend to publish this package in Alpine shortly. |
10c694a
to
9fe4df5
Compare
@@ -24,6 +24,7 @@ install: ${PROG} ${MAN} | |||
chmod ${BINMODE} ${DESTDIR}${BINDIR}/${PROG} | |||
cp -f doas.1 ${DESTDIR}${MANDIR}/man1 | |||
cp -f doas.conf.5 ${DESTDIR}${MANDIR}/man5 | |||
cp -f doas.d.5 ${DESTDIR}${MANDIR}/man5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could the installation of this file be optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how to integrate it with the build system. Open to suggestions.
What is the current status of this patch? |
Going to merge it with the next major release, I already started working on that, but I still need some time to adapt upstream changes. |
@Duncaen any chance of this finally being merged (almost 2 years later)? Alpine has it patched into their package and it would be nice to see upstream having it. Debian and Ubuntu appear to have packaged opendoas - I have an Alpine patch for cloud-init to add doas support that relies on this PR - if it was merged and a new release of doas made then I could upstream my cloud-init doas functionality... |
@Duncaen Do you need any help with adapting to upstream changes? aside from personally wanting this to avoid touching |
This adds support for an
/etc/doas.d
configuration directory as discussed in #61. It is disabled by default.Fixes #61.
cc @jirutka, @ncopa